feat(git): Add bcommits_range picker #2398

aaronkollasch · 2023-02-22T08:47:33Z

Description

fzf.vim has a feature where the :BCommits command shows only commits affecting the selected lines, if run from visual mode.

This functionality cannot be replicated in telescope with config options alone. Fzf.vim uses the arguments git log --no-patch -L start,end:file, which are not compatible with the filename that telescope appends to git_commands. (I tried some hacks like adding an argument to git_commands to make git ignore the filename argument, but they didn't work).

Type of change

~~This PR only affects bcommits behavior when run in visual mode, so I think it's a minor change but could be considered breaking.~~

This PR adds bcommits_range to show commits affecting a range of lines. bcommits_range can get the range of selected lines in visual mode, and supports an operator-pending mode to get lines from the next text object or motion. To support operators, this PR also adds a new telescope.operators module.

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Test A: Visually select lines of code, then activate the bcommits_range picker. Only commits affecting the selected lines should be shown. Check all previewer outputs to make sure they make sense.
Test B: Run test A from a subdirectory of the git directory. Behavior should be the same as before.
Test C: Run bcommits_range in normal mode, all commits for the file should be shown.
Test D: Run <cmd>Telescope git_bcommits_range first=1 last=10<CR> in normal mode. Commits affecting the first 10 lines should be shown.
Test E: Run <cmd>Telescope git_bcommits_range operator=true<CR> in normal mode followed by a motion or text object, e.g. ip for inner paragraph. All commits affecting the selected range should be shown.

Configuration:

Neovim version (nvim --version): v0.8.3
Operating system and version: macOS 13.2.1 Ventura

Notes

The line range in the file may not match the range used to filter commits, if there are uncommitted changes to the file (whether staged or not, it seems). This affects fzf.vim as well, and solving it could be tricky.
I've supported visual block mode by including a ^V in __git.lua, not sure if that's ok, or if there's a better way to detect visual mode.

Checklist:

My code follows the style guidelines of this project (stylua)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (lua annotations)

lua/telescope/builtin/__git.lua

tjdevries · 2023-03-21T17:42:26Z

What do you think about just having a separate picker for this rather than overriding the original?

For example, we could have bcommits_range that accepts a start and end range, and we could generate those by default if it's in visual mode (but you could generate those other ways if you weren't in visual mode. For example, you could get the start and end range of a function with tree-sitter and then call this to get the commits over this function).

aaronkollasch · 2023-03-21T18:33:55Z

A separate picker sounds good to me.

That would make it possible to define an operator-pending keymap for that picker. For example, I could type <bcommits_key>if or <bcommits_key>ip in normal mode to get the commits affecting a given function or paragraph. Perhaps this behavior could be enabled with an { operator = true } option? I don't know how to make operator keymaps yet, though.

Otherwise, bcommits_range with options { start = N, end = N } and auto-populating the range in visual mode makes sense to me.

aaronkollasch · 2023-03-21T22:53:25Z

I pushed a commit with a separate bcommits_range picker. My one hesitation with this is the amount of repeated code/documentation, as any updates to bcommits would be expected to propagate to bcommits_range.

aaronkollasch · 2023-03-22T02:31:35Z

I pushed a commit which adds operator mode as an option, and it works as expected. For example, <cmd>Telescope git_bcommits_range operator=true<CR> followed by if shows all commits affecting the function under the cursor.

tjdevries · 2023-03-22T13:46:05Z

Love the operator pending stuff -- my only thought is that do you think it's possible to maybe move the operator pending logic outside of this function?

Perhaps we can make a new module telescope.operators. I think this idea is pretty cool and we may want to use the concept for some other pickers.

What are your thoughts on that?

aaronkollasch · 2023-03-23T01:50:28Z

That sounds good. I've updated the PR to move the operators functionality to telescope.operators.

tjdevries · 2023-03-23T15:04:07Z

lua/telescope/operators.lua

+  vim.o.operatorfunc = "v:lua.require'telescope.operators'.operator_callback"
+  -- feed g@ to enter operator-pending mode
+  -- 'i' required for which-key compatibility, etc.
+  vim.api.nvim_feedkeys("g@", "mi", false)


do you think after running this line we should clear the operator? Or perhaps on line 7? This way we drop the reference to the picker and it can be garbage collected later, which seems like a good idea.

Maybe it should be:

operators.operator_callback = function() last_operator.callback(last_operator.opts) last_operator = { callback = function(_) end, opts = {} } end

Yeah I tried something like that, but it breaks the key g@ to re-run the last operator. Not sure if that's a big deal or not as I don't use that keymap, but I didn't see a huge reason to break it.

How important is it to garbage collect the picker reference and opts?

Ah, that makes sense.

It is kind of important, but it should be alright cause there is at most one outstanding, right?

Right, it's only a reference to the most recent picker/callback function used with operators.run_operator. At least for this picker it would just keep a reference to git.bcommits_range and its opts.

aaronkollasch · 2023-04-26T20:57:01Z

By the way, what do you think about the names of the arguments to specify the range? I'd originally used start and end but switched to first and last because the end keyword is a bit inconvenient in lua.
Would start and end make more sense?

edit: I will keep first and last due to the inconvenience of creating tables with an element named "end".

Starts operator-pending mode and shows commits in the range of lines covered by the next text object or motion

Can't use start/end, since end is an annoying keyword to use in lua and start/stop doesn't fit as well

Instead of calling bcommits

Default to a no-op callback

See nvim-telescope#2597

aaronkollasch · 2023-07-19T04:17:35Z

I've rebased this PR on master to include the changes in #2597.

jamestrew · 2023-07-19T05:53:24Z

Thanks for sticking with this^^
I'll try to review this in the coming days.

jamestrew · 2023-07-21T22:51:28Z

By the way, what do you think about the names of the arguments to specify the range? I'd originally used start and end but switched to first and last because the end keyword is a bit inconvenient in lua. Would start and end make more sense?

edit: I will keep first and last due to the inconvenience of creating tables with an element named "end".

Maybe from and to? Open to keeping first and last -- I think they work just as well

lua/telescope/builtin/__git.lua

aaronkollasch · 2023-07-22T15:36:18Z

By the way, what do you think about the names of the arguments to specify the range? I'd originally used start and end but switched to first and last because the end keyword is a bit inconvenient in lua. Would start and end make more sense?
edit: I will keep first and last due to the inconvenience of creating tables with an element named "end".

Maybe from and to? Open to keeping first and last -- I think they work just as well

from and to sound good, I think it's a bit more natural than first and last for specifying a range in a command.

jamestrew · 2023-07-22T21:35:49Z

Beautiful. Thanks again 👍

aaronkollasch · 2023-07-22T22:05:45Z

Thanks for reviewing!

aaronkollasch changed the base branch from dev to master February 22, 2023 08:56

aaronkollasch force-pushed the visual-bcommit-filter branch from 5812136 to ed6e964 Compare February 22, 2023 09:11

aaronkollasch marked this pull request as ready for review February 27, 2023 19:48

tjdevries reviewed Mar 21, 2023

View reviewed changes

lua/telescope/builtin/__git.lua Outdated Show resolved Hide resolved

aaronkollasch force-pushed the visual-bcommit-filter branch 3 times, most recently from de717f6 to 6f8b9b2 Compare March 22, 2023 02:25

aaronkollasch force-pushed the visual-bcommit-filter branch from 6f8b9b2 to bd257b7 Compare March 22, 2023 02:37

aaronkollasch changed the title ~~feat(git): Filter bcommits by selected range in visual mode~~ feat(git): Add bcommits_range picker Mar 23, 2023

aaronkollasch force-pushed the visual-bcommit-filter branch from 07227cd to 86d76e2 Compare March 23, 2023 01:47

aaronkollasch force-pushed the visual-bcommit-filter branch 3 times, most recently from 24be9dc to 77b4c4a Compare March 23, 2023 10:08

tjdevries reviewed Mar 23, 2023

View reviewed changes

aaronkollasch force-pushed the visual-bcommit-filter branch from 1ede67a to 2da39c3 Compare April 13, 2023 22:12

aaronkollasch added 8 commits July 18, 2023 23:56

Filter bcommits by selection in visual mode

4aa6a9a

Split bcommits_range into new picker

343f3f3

Add option to run bcommits_range as operator

068254f

Starts operator-pending mode and shows commits in the range of lines covered by the next text object or motion

Rename range arguments to "first" and "last"

5a15036

Can't use start/end, since end is an annoying keyword to use in lua and start/stop doesn't fit as well

Move operators functionality to new module

e5f919e

Run bcommits if no range given to bcommits_range

b8a62e3

Make bcommits_range default to current line

ef2e484

Instead of calling bcommits

Improve documentation of telescope.operators

54347c2

Add default value for last_operator

9dc1af6

Default to a no-op callback

aaronkollasch force-pushed the visual-bcommit-filter branch from 2da39c3 to 9dc1af6 Compare July 19, 2023 03:59

Update bcommits_range for detached worktrees

787098d

See nvim-telescope#2597

jamestrew self-assigned this Jul 19, 2023

jamestrew reviewed Jul 21, 2023

View reviewed changes

lua/telescope/builtin/__git.lua Outdated Show resolved Hide resolved

aaronkollasch added 2 commits July 22, 2023 11:39

Rename range arguments to "from" and "to"

4591061

Move shared bcommits picker into single function

527caa1

jamestrew merged commit e7e6492 into nvim-telescope:master Jul 22, 2023
6 checks passed

aaronkollasch deleted the visual-bcommit-filter branch July 22, 2023 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(git): Add bcommits_range picker #2398

feat(git): Add bcommits_range picker #2398

aaronkollasch commented Feb 22, 2023 •

edited

Loading

tjdevries commented Mar 21, 2023

aaronkollasch commented Mar 21, 2023

aaronkollasch commented Mar 21, 2023

aaronkollasch commented Mar 22, 2023

tjdevries commented Mar 22, 2023

aaronkollasch commented Mar 23, 2023

tjdevries Mar 23, 2023

aaronkollasch Mar 23, 2023 •

edited

Loading

tjdevries Apr 26, 2023

aaronkollasch Apr 26, 2023

aaronkollasch commented Apr 26, 2023 •

edited

Loading

aaronkollasch commented Jul 19, 2023

jamestrew commented Jul 19, 2023

jamestrew commented Jul 21, 2023

aaronkollasch commented Jul 22, 2023

jamestrew commented Jul 22, 2023

aaronkollasch commented Jul 22, 2023

feat(git): Add bcommits_range picker #2398

feat(git): Add bcommits_range picker #2398

Conversation

aaronkollasch commented Feb 22, 2023 • edited Loading

Description

Type of change

How Has This Been Tested?

Notes

Checklist:

tjdevries commented Mar 21, 2023

aaronkollasch commented Mar 21, 2023

aaronkollasch commented Mar 21, 2023

aaronkollasch commented Mar 22, 2023

tjdevries commented Mar 22, 2023

aaronkollasch commented Mar 23, 2023

tjdevries Mar 23, 2023

Choose a reason for hiding this comment

aaronkollasch Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

tjdevries Apr 26, 2023

Choose a reason for hiding this comment

aaronkollasch Apr 26, 2023

Choose a reason for hiding this comment

aaronkollasch commented Apr 26, 2023 • edited Loading

aaronkollasch commented Jul 19, 2023

jamestrew commented Jul 19, 2023

jamestrew commented Jul 21, 2023

aaronkollasch commented Jul 22, 2023

jamestrew commented Jul 22, 2023

aaronkollasch commented Jul 22, 2023

aaronkollasch commented Feb 22, 2023 •

edited

Loading

aaronkollasch Mar 23, 2023 •

edited

Loading

aaronkollasch commented Apr 26, 2023 •

edited

Loading